Support for response multiValueHeaders locally #842
Conversation
|
Hi @jfuss, could you let me know what else is required for this? Thanks! |
|
@martysweet Apologies on the slow response here. I am talking a look at this now. |
|
|
||
| # Get the first part of the content-type header, to allow for extras such as text/html; charset=utf-8 | ||
| content_type = lamba_response_headers['Content-Type'].split(";", 1)[0] | ||
| best_match_mimetype = flask_request.accept_mimetypes.best_match([content_type]) |
There was a problem hiding this comment.
How did you validate this is the correct behavior?
There was a problem hiding this comment.
A MIME type should contain only type/subtype.
https://developer.mozilla.org/en-US/docs/Web/HTTP/Basics_of_HTTP/MIME_types
However, the Content-Type header contains the MIME, then potentially extra charset information:
https://www.ietf.org/rfc/rfc2045.txt (page 12)
In the Augmented BNF notation of RFC 822, a Content-Type header field
value is defined as follows:
content := "Content-Type" ":" type "/" subtype
*(";" parameter)
; Matching of media type and subtype
; is ALWAYS case-insensitive.
Passing this extra charset information into flask_request.accept_mimetypes.best_match results in Flask returning 500.
There was a problem hiding this comment.
@martysweet I should have been a little clearer. Does this behavior match API Gateway's? Meaning do they only look at the first Content-Type Header or do they compare with the list of all headers? Did we match documentation or what is observed from the service?
There was a problem hiding this comment.
@jfuss Ah understood. Looking into it, this might be more complex than just looking at Content-Type, though a bit outside of this PR.
So your question is, if multiple Content-Type Headers are returned to API Gateway, which one is used for the comparison against the binary types list?
I can't find anything in the docs, will have to run some tests.
Docs regarding Accept and Content-Type (which doesn't answer the question).
https://docs.aws.amazon.com/apigateway/latest/developerguide/api-gateway-payload-encodings-configure-with-console.html
https://docs.aws.amazon.com/apigateway/latest/developerguide/api-gateway-payload-encodings-workflow.html
|
@martysweet I really would love for this to work. when I'm testing it using both your branch here, and apigw, they do act different. this code creates a comma-seperated list in a single header (which chrome, at least, doesnt like), and apigw creates multiple headers with the same key (in my tests I'm using set-cookie, but I'm sure it's for anything that it applies to) |
@everyonce, this isn't true, actually. |
|
Closing in favour of #1166 |
Issue #, if available:
Addresses is #830, supporting multiValueHeaders locally.
Description of changes:
Merges lamdba response headers and multiValueHeaders in accordance with guidance from here.
https://docs.aws.amazon.com/apigateway/latest/developerguide/set-up-lambda-proxy-integrations.html#api-gateway-simple-proxy-for-lambda-output-format
Within base64 logic, splits Content-Type by ;, to prevent issues with malformed Content types.
content_type = lamba_response_headers['Content-Type'].split(";", 1)[0]_should_base64_decode_bodychange - Guidance neededChecklist:
make prpassesPlease let me know what else is required/missing! 馃槃 This PR plays well with #741, which is for requests.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.